Repository navigation
[WTForms] Update stubs for 3.3.0 - #16604
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| def iter_choices(self) -> Iterator[_FullChoice]: ... | ||
| def _iter_choices_normalized(self) -> Iterator[Choice]: ... | ||
| def has_groups(self) -> bool: ... | ||
| def iter_groups(self) -> Iterator[_FullGroupedChoices]: ... | ||
| def _iter_groups_normalized(self) -> Iterator[tuple[str, list[Choice]]]: ... |
There was a problem hiding this comment.
I'm a little conflicted about exposing these internal normalization helpers, that are likely to disappear in 3.4.0
Yielding a plain tuple is deprecated and the Select widget will check for that and emit a DeprecationWarning, but during this deprecation period there's no guarantees about whether or not you will get a plain tuple, or the Choice named tuple if you're using custom subclasses that haven't been updated yet.
So either we change iter_choices and iter_groups to the new signatures and you get a hard type error for something that's supposed to be just a deprecation warning and a false negative on widgets that expect to get Choice objects, when in fact they still could receive plain tuples, or we leave it like this and you cannot update your own Widgets to rely on the presence of the named Choice attributes, without exposing this internal helper, or doing the conversion yourself.
There was a problem hiding this comment.
I have absolutely no experience with WTForms, so I would like to leave the ultimate decision to you, but to me it sounds the second solutions is the better one, since it avoids the false negatives you mentioned.
Johann150
left a comment
There was a problem hiding this comment.
Hi there, got started on updating this after mypy complained. But then gladly found this PR before I got too deep in the weeds. ;)
I noticed a few small things missing that I picked up from the changelog, please see below.
Co-authored-by: Johann150 <johann@qwertqwefsday.eu>
This comment has been minimized.
This comment has been minimized.
srittau
left a comment
There was a problem hiding this comment.
Thanks, a few comments, mostly about uncommented Anys.
| _FieldT = TypeVar("_FieldT", bound=Field) | ||
| _LegacyChoice: TypeAlias = tuple[Any, str] | tuple[Any, str, dict[str, Any]] | ||
| _Choice: TypeAlias = SelectChoice | _LegacyChoice | ||
| # it's too difficult to get type safety here due to to nested partially invariant collections | ||
| _GroupedChoices: TypeAlias = dict[str, Any] # Any should be Collection[_Choice] | ||
| _FullChoice: TypeAlias = tuple[Any, str, bool, dict[str, Any]] # value, label, selected, render_kw | ||
| _LegacyChoices: TypeAlias = Iterable[_LegacyChoice] | _GroupedChoices | ||
| _LegacyChoicesCallback: TypeAlias = Callable[[], Sequence[_LegacyChoice] | _GroupedChoices] | ||
| _SelectChoicesCallback: TypeAlias = Callable[[Any, _FieldT], Sequence[SelectChoice]] | Callable[[], Sequence[SelectChoice]] | ||
| _FullChoice: TypeAlias = Choice | tuple[Any, str, bool, dict[str, Any]] # value, label, selected, render_kw |
There was a problem hiding this comment.
There are a few undocumented Anys here. Could you add a short comment, explaining them? (dict[str, Any] doesn't need explanation, and it can also be something simple like the comment for _FullChoice.)
| def __call__(self, **kwargs: Any) -> Markup: ... | ||
| def datalist(self, **kwargs: Any) -> Markup: ... |
There was a problem hiding this comment.
Could you also add a short comment what happens with the kwargs (to explain the Any)?
| def append_entry(self, data: Any = ...) -> _BoundFieldT: ... | ||
| def pop_entry(self) -> _BoundFieldT: ... | ||
| def insert_entry(self, index: int, data: Any = ...) -> _BoundFieldT: ... |
There was a problem hiding this comment.
Same here: What's data/why is Any used?
| def __call__(self, field: FormField[Any], **kwargs: object) -> Markup: ... | ||
|
|
||
| class DataListWidget: | ||
| def __call__(self, datalist: DataList, field: Field | None = None, **kwargs: Any) -> Markup: ... |
| input_type: str | ||
| validation_attrs: list[str] | ||
| def __init__(self, input_type: str | None = None) -> None: ... | ||
| def __call__(self, field: Field, **kwargs: Any) -> Markup: ... |
| def __init__(self, input_type: str | None = None) -> None: ... | ||
| def __call__(self, field: Field, **kwargs: Any) -> Markup: ... | ||
| @staticmethod | ||
| def html_params(**kwargs: object) -> str: ... |
There was a problem hiding this comment.
This was copied from Input which defines it the same way.
Since this is mostly a convenient re-export of the function with the same name defined in the same module, I doubt anyone would want to override it in a subclass, so object seemed most appropriate, since everything just gets coerced to Markup via markupsafe.escape, which is a str subclass and it'll either take the output of the __html__ method if there is one, or just convert the object to a string via str() before escaping, apart from True and False, which are special cased.
So the function truly accepts any object, rather than an indeterminate set of objects like e.g. json.dumps would, although one could argue that apart from numeric and string types, only types implementing __html__ truly do make sense to pass in, but that view seems too strict to me for general purpose stubs.
There was a problem hiding this comment.
Sounds like a good reason. I wouldn't mind a brief comment ("kwargs go through str"), just to keep me and my gold fish brain from asking the same question again in two months.
| def __call__(self, field: SelectFieldBase, **kwargs: object) -> Markup: ... | ||
| @classmethod | ||
| def render_option(cls, value: object, label: str, selected: bool, **kwargs: object) -> Markup: ... | ||
| def render_option(cls, choice: Choice, **kwargs: object) -> Markup: ... |
There was a problem hiding this comment.
Optional, since you didn't change it, but can we apply the object/Any explanation/changes here, too?
|
|
||
| @type_check_only | ||
| class _DataListWidget(Protocol): | ||
| def __call__(self, datalist: DataList, /, *, field: Field | None = None, **kwargs: Any) -> Markup: ... |
| widget: _DataListWidget | None = None, | ||
| ) -> None: ... | ||
| def iter_choices(self, field: Field | None = None) -> list[DataListChoice]: ... | ||
| def __call__(self, field: Field | None = None, **kwargs: Any) -> Markup: ... |
This comment has been minimized.
This comment has been minimized.
| with_table_tag: bool | ||
| def __init__(self, with_table_tag: bool = True) -> None: ... | ||
| def __call__(self, field: FormField[Any], **kwargs: object) -> Markup: ... | ||
| def __call__(self, field: FormField[Any], **kwargs: Any) -> Markup: ... |
There was a problem hiding this comment.
Should this also be _RenderArg?
There was a problem hiding this comment.
Indeed it should, looks like I missed one
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
|
Thanks everyone :)
|
Release: https://pypi.org/pypi/WTForms/3.3.0
Repository: https://github.com/pallets-eco/wtforms
Typeshed stubs: https://github.com/python/typeshed/tree/main/stubs/WTForms
Changelog: https://wtforms.readthedocs.io/changes
Diff: pallets-eco/wtforms@3.2.2...3.3.0
Closes: #16605
Most of the major changes are related to how choices are passed to select fields, the old style has been deprecated and will be removed in 3.4.0, this makes the types a little unwieldy now, but they should simplify again later.